Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[NEW] Modal #9092

Merged
merged 11 commits into from
Dec 13, 2017
Merged

[NEW] Modal #9092

merged 11 commits into from
Dec 13, 2017

Conversation

karlprieb
Copy link
Contributor

@RocketChat/core

Closes #8775

Our new modal system :) This PR implement that and remove sweetalert :)

2017-12-12 15 09 25

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-9092 December 12, 2017 18:00 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-9092 December 12, 2017 18:03 Inactive
geekgonecrazy
geekgonecrazy reviewed Dec 12, 2017
View reviewed changes
packages/rocketchat-livechat/app/client/lib/commands.js
@@ -1,4 +1,4 @@
/* globals LivechatVideoCall, Livechat */
/* globals LivechatVideoCall, Livechat, swal */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it still needed to add swal to globals when actually removing it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Livechat widget still uses sweetAlert

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 makes sense I just saw the adding in a bunch of files so was checking :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@geekgonecrazy
Copy link
Member

geekgonecrazy commented Dec 12, 2017 •
edited
Loading

can we add some key press love up in here?

//psuedo code (not sure which element to bind on)
onkeydown = function (event) { 
  if (event.which == 13) {
    modal.confirm(true);
  }

  if (event.which == 27) {
    modal.close();
  }
}

otherwise it feels painful to share files 😛

graywolf336
graywolf336 suggested changes Dec 12, 2017
View reviewed changes
Copy link
Contributor

@graywolf336 graywolf336 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add support for the ESC and ENTER keys, otherwise this will disrupt the flow of many people who rely on them on an hourly basis per day.

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-9092 December 13, 2017 19:01 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-9092 December 13, 2017 19:02 Inactive
@karlprieb
Copy link
Contributor Author

@graywolf336 @geekgonecrazy I added ESC and Enter events now.

sampaiodiego
sampaiodiego requested changes Dec 13, 2017
View reviewed changes
packages/rocketchat-theme/client/imports/components/modal.css Outdated

&__content-icon {
font-size: 6rem;
margin: 1.2rem;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change for tabs

packages/rocketchat-theme/client/imports/components/modal.css Outdated

white-space: nowrap;
&__content-text {
line-height: 1.5;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change for tabs

packages/rocketchat-theme/client/imports/components/modal.css Outdated
&__content-error {
display: none;
width: 100%;
text-align: center;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change for tabs

packages/rocketchat-ui/client/views/app/popover.js Outdated
if (['channel', 'group', 'direct'].includes(FlowRouter.getRouteName()) && (Session.get('openedRoom') === rid)) {
FlowRouter.go('home');
}

RoomManager.close(rid);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add this back pls

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-9092 December 13, 2017 19:12 Inactive
@geekgonecrazy
Copy link
Member

@karlprieb you rule! Balance and Order has been restored to the universe. 😄

@sampaiodiego sampaiodiego added this to the 0.60.0 milestone Dec 13, 2017
graywolf336
graywolf336 reviewed Dec 13, 2017
View reviewed changes
.eslintrc Outdated
@@ -122,6 +122,7 @@
"Logger" : false,
"Match" : false,
"Meteor" : false,
"modal" : false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be aligned with the others?

@sampaiodiego sampaiodiego merged commit 23737a7 into develop Dec 13, 2017
@sampaiodiego sampaiodiego deleted the rc-modal branch December 13, 2017 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geekgonecrazy geekgonecrazy geekgonecrazy left review comments

@graywolf336 graywolf336 graywolf336 approved these changes

@sampaiodiego sampaiodiego sampaiodiego approved these changes

@rodrigok rodrigok Awaiting requested review from rodrigok

@ggazzo ggazzo Awaiting requested review from ggazzo

@MartinSchoeler MartinSchoeler Awaiting requested review from MartinSchoeler

Assignees
No one assigned
Labels
type: release candidate bug
Projects
None yet
Milestone
0.60.0
Development

Successfully merging this pull request may close these issues.

New Modal Component
5 participants
@karlprieb @geekgonecrazy @graywolf336 @sampaiodiego @engelgabriel